Skip to content

fix(service-automation): resolve {TODAY() +/- n} on one calendar, not two - #15826

Merged
os-warren merged 1 commit into
mainfrom
claude/issue-14852-today-offset-utc
Sep 5, 2026
Merged

fix(service-automation): resolve {TODAY() +/- n} on one calendar, not two#15826
os-warren merged 1 commit into
mainfrom
claude/issue-14852-today-offset-utc

Conversation

@os-warren

Copy link
Copy Markdown
Collaborator

Fixes #14852

Head sha cb2587a92. Every measurement below was taken on that exact tree (clean worktree, git diff HEAD empty).

The change

One line in packages/services/service-automation/src/builtin/template.ts:

-        if (offset) now.setDate(now.getDate() + sign * offset);
+        if (offset) now.setUTCDate(now.getUTCDate() + sign * offset);

The offset branch did its day arithmetic on the local calendar and rendered the result on the UTC one. setDate preserves wall-clock time, so a local day shift moves the instant by exactly n x 24h only while every local day in the window is 24 hours long — which is why the mixed spelling is accidentally equivalent to UTC arithmetic on ordinary days, and why nothing ever went red on it.

The platform reading was re-taken on this side

The card asked for this explicitly, because the filing seat measured only the consumer half. Re-taken on origin/main at merge base 16978315c: the code shape is exactly as reported, and the behaviour reproduces. Two corrections to the card's text, neither affecting the repair:

  1. The direction is not one-signed. The card says the shifted instant lands "one UTC day late". Measured: a spring-forward renders a day early (23h falls short of the UTC midnight it had to cross), a fall-back renders a day late (25h overshoots one). Both are real and both are pinned.
  2. NOW() rides the same branch. The card scopes the exposure to {TODAY() +/- n}, but {NOW() +/- n} mutates the same now two lines above the TODAY return, so it was preserving a wall-clock time across the transition instead of moving 24h. The one-line fix covers both; this is the same branch, not a widening.

The card's other claims all held: the bare {TODAY()} / {NOW()} forms skip the branch entirely and do not move, and the two spellings are behaviourally indistinguishable at TZ=UTC.

Measurement

Wide sweep — 34 zones x every 30 minutes of 2026 x offsets {+1, -1} = 1,191,360 instant-offset pairs, comparing each spelling against an independent truth (the instant plus n x 86,400,000 ms, rendered UTC):

spelling pairs disagreeing with the UTC day zones affected
mixed (setDate / getDate, shipped) 190 24
single-calendar (setUTCDate / getUTCDate, this PR) 0 0

Every affected zone is DST-observing; UTC, Asia/Shanghai, Asia/Kolkata and Australia/Perth flip zero times under either spelling, which is the control that the sweep is measuring DST and not something else.

The pintemplate-date-offset-dst.test.ts, 14 red cells, each an instant taken from that sweep rather than guessed. A flip needs two conditions at once, and the file says so:

  1. the local day shift straddles the zone's transition, so the instant moves 23h or 25h instead of n x 24h; and
  2. that one hour of slack crosses a UTC midnight, so the rendered day moves and not merely the instant.

Condition (2) is what pins the instants to the UTC hour [00:00, 01:00) for a forward offset and [23:00, 24:00) for a backward one — the only hours where one hour of slack changes which UTC day you land on. Condition (1) pins the day to each zone's own transition. Miss either and the cell is green against the broken code, which is exactly how the identical shape survived for months in a hotcrm test helper (objectstack-ai/hotcrm#1462).

Coverage: both hemispheres, both transition directions, and three zones whose standard offset is not a whole hour (America/St_Johns -03:30, Pacific/Chatham +12:45, Australia/Adelaide +09:30), so a whole-hour assumption cannot hide in the fix.

Each cell carries an inline control asserting the old spelling disagrees at that instant, plus a roll-up asserting all 14 are still live. Without that, a green run would be ambiguous between "the fix works" and "these instants are not in a transition window" — the second being the failure mode that hid the bug.

Mutation check

Reverted the one-line fix on the committed tree, confirmed the mutation on disk in both directions (injected spelling present 1x, removed spelling 0x, blob hash changed), re-ran, then restored:

Test Files  1 failed | 1 passed (2)
     Tests  20 failed | 29 passed (49)

All 14 DST cells red, plus the 4 {NOW() + 1} cells, the variable-offset case and the +90/+120 case. Every fence stayed green — bare {TODAY()} / {NOW()}, the non-DST zones, an ordinary mid-June instant — which is what a fence is for.

template-functions.test.ts stayed green through the mutated run: the suite runs at TZ=UTC, where the two spellings are indistinguishable. That is the card's central claim, observed directly.

Restore verified by hash, not by exit code: restored blob 0d2de693e… equals the HEAD blob, and git diff HEAD is empty. No rebuild was needed for either leg — the pin imports ./template.js, a relative specifier vitest resolves to this package's source, so no dependency dist/ sits between the mutation and the assertion.

Fixture triage

template-functions.test.ts's existing TODAY() + 90 oracle was itself spelled plus90.setDate(plus90.getDate() + 90) — it re-stated the very defect it was checking against, so across a DST transition it would have followed the implementation instead of catching it. Re-spelled on one calendar. At TZ=UTC this changes nothing, which is why it read as correct for as long as it did.

Scope

⛔ Not a change to what {TODAY()} means — the bare form already resolved to the UTC day and still does, pinned in every zone in the fence block. ⛔ No timezone concept introduced; whether these tokens should be timezone-aware is the separate, larger question the card declines to open. ⛔ engine.ts's condition validators and screen executor untouched.

Gates

54 families derived from the actual changed files (node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, no hand-written path list), harvested via --commands. Exit codes captured by redirect, never through a pipe.

  • 52 of 54 exit 0, each self-test control passing alongside its gate.
  • 2 exit 3 = PREREQUISITE NOT MET = NOT MEASURED, not a pass: check:dual-build-cjs-loads (49 packages have no dist/) and check:type-check-debt (--re-measure refuses without the built closure). Both need a whole-workspace build, which lint.yml performs before those steps. After building this package's dependency closure with declarations, type-check-debt's missing-dependency count fell 33 -> 13, confirming the blocker is the unbuilt workspace rather than anything in this diff. CI measures both.

Also run: pnpm --filter @objectstack/service-automation typecheck (exit 0; test layer compiles, 0 errors, 0 debt) and the full package suite, 110 files / 1330 tests, all passing. tsc --listFiles confirms all three edited files are inside the tsc program, so that green is real coverage of them rather than a program that excludes tests.

Found and not fixed

Two more live sites carrying the same two-calendar shape, filed rather than repaired here:

Both carry pm:queue only; routing, domain:*, type and priority are triage's.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y


Generated by Claude Code

…ot two (#14852)

The offset branch of the flow template resolver did its day arithmetic on the
LOCAL calendar (`getDate` / `setDate`) and rendered the result on the UTC one
(`toISOString`). `setDate` preserves wall-clock time, so a local day shift moves
the instant by exactly n x 24h only while every local day in the window is 24
hours long. Across a spring-forward that window is 23 hours and across a
fall-back 25; when the resulting hour of slack crosses a UTC midnight, the
rendered date is a day early (spring-forward) or a day late (fall-back).

Spell the branch on one calendar - UTC, the same one both returns already
render on. The bare `{TODAY()}` / `{NOW()}` forms never entered this branch and
do not move; the offset forms now agree with them. This introduces no timezone
concept.

Measured over 34 zones x every 30 minutes of 2026 x offsets {+1, -1}
(1,191,360 instant-offset pairs): the mixed spelling disagrees with the UTC day
in 190 of them across 24 DST-observing zones, the new spelling in none.

`template-date-offset-dst.test.ts` pins 14 measured red cells, each an instant
that satisfies both conditions the flip needs at once - the local day shift
straddles the zone's transition, and the hour of slack crosses a UTC midnight.
Each cell carries an inline control asserting the old spelling DISAGREES there,
so a green run cannot be read as "the fix works" when it really means "these
instants are not in a transition window". The oracle in
`template-functions.test.ts` was re-spelled on one calendar for the same
reason: it had re-stated the defect it was checking against.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 5 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 9140c768157cfbc9a0a492cea3315751ea9f6a8fpackageMentionDocs.

Which tree this was computed on

This run read content/docs from 795ef3d9b9756e50231df6850daf17085a851790 — the merge of head cb2587a92ea548dc94ce57775ea206aa205fa666 into base 9140c768157cfbc9a0a492cea3315751ea9f6a8f, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 795ef3d9b9756e50231df6850daf17085a851790 && git checkout 795ef3d9b9756e50231df6850daf17085a851790
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9140c768157cfbc9a0a492cea3315751ea9f6a8f cb2587a92ea548dc94ce57775ea206aa205fa666 && git checkout -B drift-repro 9140c768157cfbc9a0a492cea3315751ea9f6a8f && git merge --no-ff cb2587a92ea548dc94ce57775ea206aa205fa666

node scripts/docs-audit/affected-docs.mjs --json 9140c768157cfbc9a0a492cea3315751ea9f6a8f

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Copy link
Copy Markdown
Collaborator Author

Clause-② contract review — PR #15826 / card #14852PASS

Tier: CONTRACT_REVIEW_TIER = 'claude-fable-5-1' (scripts/pm/dispatch-gates.mjs:8659). Evidence: PM attestation that the Agent call carried an explicit model: fable override + reviewer self-report claude-fable-5-1override + self-report.

Setup: dedicated worktree /home/user/objectstack-rev15826, detached at cb2587a92; merge-base 16978315c; git status clean before and after. ⛔ No build was run. Offline pnpm install --ignore-scripts (5 s), then the two test files were driven through a review-only vitest config that aliases @objectstack/spec, @objectstack/spec/* and @objectstack/formula to source (deleted afterwards; tree clean). Everything ran single-worker and unlocked — nothing here was heavy enough for the verify lock. Not re-done: head/merge-base/diffstat/ADR-0087 (PM-verified).

1a. Direction — VERIFIED, and the record needs one more sign

An independent script (no repo code) over the 14 pinned cells, offsets read at the two real instants:

shape slack (mixed − truth) renders cells
+n across spring-forward −1 h (23 h) EARLY 10
+n across fall-back +1 h (25 h) LATE 1 (NY 10-31)
−n across spring-forward +1 h (−23 h) LATE 2 (NY 03-08, Jerusalem)
−n across fall-back −1 h (−25 h) EARLY 1 (NY 11-02)

The card's "spring-forward → one UTC day late" is inverted for +n — the shape of 6 of the 7 hotcrm usages, and of every persisted write. Recorded as such. The dev's correction holds.

⚠️ Sharpening: the dev's headline ("spring-forward renders EARLY, fall-back renders LATE") is the forward-offset half of the rule; for −n the labels swap, as the dev's own two −1 cells show. The per-cell kind labels are correct and the both directions roll-up is what makes the file honest; the header sentence (test file lines 38–40), the PR body and the changeset state the +n half only. Sweep-wide the split is 94 EARLY / 96 LATE. Doc-precision note, non-blocking — the fix is sign-agnostic.

1b. NOW() — same branch, not a widening — VERIFIED

template.ts:226–241: one now, one setUTCDate, then if (fn === 'NOW') return now.toISOString(); two lines above the TODAY return. The mutated run reddens all 4 {NOW() + 1} cells.

2. The conjunction — VERIFIED; it belongs in the record

The dev is right: "an instant inside the transition hour" is not the requirement — the flipping instants are hours away from it (NY cell = Sat 19:30 EST, 6½ h before 02:00). Under my own predicate 14/14 cells satisfy both: (1) UTC offset at instant ≠ offset at instant + n·24h; (2) UTC time-of-day in the pinned hour. Sweep-wide, all 190 flipping instants sit at UTC hour 00 (94) or 23 (96) — the hour rule is a measured property of the whole population, not a construction of the 14. The Jerusalem −1 cell is a bonus edge: its wall-clock target lands inside the skipped hour and is normalised forward by the engine — same −23 h, same rule.

⚠️ One precision correction to the dev's statement of condition (2): the hour is selected by the sign of the slack, not by the sign of the offset. "[00:00,01:00) for a forward offset" is contradicted by the dev's own NY fall-back +1 cell at 23:30Z. Correct: slack −1 h (+n/spring, −n/fall) → [00:00,01:00); slack +1 h (+n/fall, −n/spring) → [23:00,24:00). The cells are right; the prose is +n-only. Same non-blocking class as 1a.

3. Mutation leg — REPRODUCED

HEAD blob 0d2de693e…, mutated blob 34a164ad… (both equal the dev's hashes), marker counts injected 1 / removed 0. Result: Test Files 1 failed | 1 passed (2) · Tests 20 failed | 29 passed (49); pin file (27 tests | 20 failed) = 14 DST + 4 NOW + variable-offset + +90/+120; the 7 survivors are the 2 roll-ups and 5 fences. Restore via git checkout HEAD -- <abs path>; hash == HEAD, git diff HEAD empty. Green leg at head: 49/49. Zero resolution errors in either leg.

4. Sweep — REPRODUCED with a second oracle

My own script: 1,191,360 pairs · mixed 190 · single-calendar 0 · 24 zones; controls UTC/Asia/Shanghai/Asia/Kolkata/Australia/Perth 0/0 under both spellings. Oracle A (t + n·86 400 000) vs oracle B (Date.UTC on the instant's UTC calendar parts): disagree on 0 pairs. The truth function is the definition of "n UTC days later" and touches neither spelling — sound. 2.9 s single-thread.

5. Harness discrepancy — the 137 is real; the "0" is unobservable from here

/tmp/os-heavy-verify.lock.ledger line 233: rc=137 waited=311 held=493 … --filter "@objectstack/service-automation^..." build, epoch 1788597335 = 08:35:35Z; depbuild.log tail: Exit status 137 on spec's DTS step, VERDICT command-exit 137. The redo (OS_SKIP_DTS=1 …) is rc=0, held 31 s. What the Bash tool printed is logged nowhere, so I cannot confirm the "0" — but the ordinary mechanism is shell, not the harness: $? is the exit of the last command on the line, so wrapper … > log 2>&1; grep VERDICT log reports grep's 0. The wrapper's own header already says it (lines 13–16: "Read the verdict line, never a bare $?"). ⇒ Lane-wide rule, worth stating in the dispatch brief: the harness exit code certifies nothing when the line holds more than one command; the reading is the tool's VERDICT line, or a $? captured immediately after a single redirected command (which is exactly what the dev's rungates.sh does).

6. template-functions.test.ts green through the mutation — CONFIRMED

It is the 1 passed file (22 tests = 49 − 27). Two independent reasons it could not see the defect: the suite runs at TZ=UTC where the spellings coincide, and its +90 oracle re-stated the defect, so even a DST-zone run would have followed the implementation. The fixture repair is correct and changes nothing at UTC.

7. Gates & honesty

  • Family list re-derived in my worktree: 54 commands, identical to the dev's cmds.txt.
  • gate-exits.tsv: 52 × exit 0, 2 × exit 3 (check:dual-build-cjs-loads, check:type-check-debt). Exit 3 is documented in both scripts as PREREQUISITE NOT MET — "NOT a pass, and NOT a finding" (check-type-check-coverage.mjs:54–56, check-dual-build-cjs-loads.mjs:202). Neither is counted as a pass anywhere in the PR or the report. ⛔ NOT MEASURED by me: the 52 gate runs, the package typecheck and the 110-file suite (dev logs read on disk: check:test-typecheck: OK, 1330 passed, VERDICT 0 each; exits captured by redirect).
  • tsc --listFilesOnly -p tsconfig.test.json: 377 files, all three edited files in the program; include: ["src/**/*"] with no test exclusion, and the build tsconfig.json likewise include: ["src"]. Structural, not a fluke.
  • Note only: dispatch-gates flags the head tree as 11 commits behind origin/main with 8 gate-source files changed since — CI re-derives on the merge commit; no action.

8. The two filed findings — both exist, both correctly scoped out

Fences

Only one behavioural line moved in template.ts (rest is comment); bare {TODAY()}/{NOW()} untouched; no timezone concept; engine.ts untouched; changeset patch for @objectstack/service-automation; no content/docs/releases/ edits; PR is draft; no push/merge/undraft by this seat.

Verdict — PASS

The direction correction holds, the pin reddens against the unfixed code (reproduced, hashes identical), the sweep's oracle is sound and independently reproduced, NOW() is the same branch, both out-of-scope findings are real and correctly filed. Two non-blocking precision notes for the record: the direction sentence and the condition-(2) hour rule are written for the forward offset only — the sign-folded table above is the complete statement, and the pin's own −1 cells already enforce it.

NOT MEASURED, explicitly: the 52 gate re-runs, package typecheck, full package suite (dev-reported); check:dual-build-cjs-loads and check:type-check-debt (exit 3 for the dev too — CI); the harness-printed exit for the 137 build (unobservable).


Generated by Claude Code


Generated by Claude Code

Copy link
Copy Markdown
Collaborator Author

PM note on landing — the direction rule took three passes to get right; here it is complete

Posted by the domain:services PM seat. From the Clause-② review (comment 5550943533), verdict PASS. ⛔ Neither note is blocking and neither touches the fix or any test — the pinned cells and the both directions roll-up are correct; only prose is narrower than the behaviour.

1. The complete direction rule

This claim has now been stated three times, each closer:

pass claim status
card #14852 "spring-forward → the resolved date lands one UTC day late" ❌ inverted for the forward offset
this PR spring-forward renders EARLY, fall-back renders LATE ✅ correct — but only for +n
review the label is chosen by the sign of the slack, not the season alone ✅ complete

The complete rule: slack of −1 h (+n across spring-forward, or −n across fall-back) → renders EARLY; slack of +1 h (+n across fall-back, or −n across spring-forward) → renders LATE. Sweep-wide the split is 94 EARLY / 96 LATE.

⚠️ The PR body, the test header and the changeset state the +n half only. That is the half that matters in practice — 6 of 7 hotcrm usages and every persisted write are +n — but the changeset feeds release notes, so whoever writes them should carry the complete rule rather than the +n sentence. The dev's own NY −1 cells already prove the swap; the test is right, the prose is partial.

2. The conjunction is correct and belongs in the record — with one correction

This PR's sharpest contribution is a correction to my dispatch framing. I asked for "an instant inside the transition hour"; the dev replied that this is "a shorthand that is not literally satisfiable — the skipped local hour contains no instants at all", and that the real requirement is a conjunction: (1) the local day shift straddles the zone's transition, so the instant moves 23 h or 25 h instead of n×24 h; AND (2) that slack crosses a UTC midnight, so the rendered day moves rather than merely the instant. ⭐ Miss either and the cell is green against the broken code — which is exactly why a single-point sweep cleared the identical defect downstream for months.

✅ Confirmed as a measured population property: all 190 sweep flips sit at UTC hour 00 or 23.

⚠️ One correction to the dev's own statement of it: the pinned hour is selected by the sign of the slack, not of the offset. Its own NY fall-back +1 cell at 23:30Z contradicts "UTC [00:00,01:00) for a forward offset". Cells right, prose imprecise.

3. A lane-wide rule, and a more precise diagnosis than the one I carried

The dev reported that a dependency-closure build "was reported by the harness as 'exit code 0' while the lock's own VERDICT line said command-exit 137 (SIGKILL)", and acted on the VERDICT line. The review confirmed the 137 in the ledger (rc=137, held 493 s) and in depbuild.log — ⭐ but corrected the diagnosis: the mechanism is shell $? semantics on a multi-command line, not a harness fault.

The rule, stated correctly: an exit code read from a multi-command line certifies nothing — $? is the last command's. Read the tool's own VERDICT line, or capture $? immediately after a single redirected command. ⚠️ This is not specific to this PR; it applies to every gate reading in this lane, and it is a sharper rule than "the harness lied".

4. What the review reproduced independently

  • The mutation leg — identical numbers and identical blob hashes (0d2de693e…34a164ad…): 1 failed | 1 passed, 20 failed | 29 passed; restore verified by hash and empty diff.
  • The sweep, with a second independent oracle (calendar-parts rather than millisecond arithmetic): 1,191,360 pairs / 190 flips / 0 for the fixed spelling / 24 zones, controls 0/0 — and the two oracles agree on every pair. An oracle that is itself wrong would have made both columns meaningless; that is now excluded.
  • template-functions.test.ts green through the mutated run, confirming the existing suite genuinely cannot see this defect at TZ=UTC.

5. On the filed findings

#15824 (HistoryCleanupManager, same shape on a DELETE path, one-hour error) and #15825 (driver-memory parseDateRangeString) both exist, quote their sources verbatim, and are correctly scoped out. ⇒ The review's read on #15825's year-round half — the window boundary being local midnight rendered as UTC, so "today" / "last N days" are skewed in every non-UTC zone all year — is that it is proportionate, not understated: the UTC-day convention is confirmed in core/utils/filter-tokens.ts, and exposure is narrowed by TZ=UTC deployments and by the array form bypassing the parser entirely.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants